Skip to content

fix: prevent client from bypassing random spawn selection πŸ›‘οΈ#4428

Merged
evanpelle merged 1 commit into
mainfrom
fix/random-spawn-bypass
Jun 27, 2026
Merged

fix: prevent client from bypassing random spawn selection πŸ›‘οΈ#4428
evanpelle merged 1 commit into
mainfrom
fix/random-spawn-bypass

Conversation

@FloPinguin

Copy link
Copy Markdown
Contributor

Description:

When random spawn mode is active, players are supposed to receive randomly chosen spawns rather than choosing their own. However, SpawnExecution.getSpawn() checks center !== undefined first, which means if a player manually injects coordinates into the spawn intent (bypassing the client-side UI guard), the random selection logic is completely bypassed and the player gets their chosen coordinates.

This was fully exploitable in singleplayer (where no pre-created SpawnExecution objects exist) and was a defense-in-depth gap in multiplayer (relying on execution order of pre-created spawns to block it via the hasSpawned() guard).

The fix forces center to undefined in getSpawn() when random spawns are enabled, ensuring the random selection code path is always taken regardless of what the client sends.

Changes:

  • src/core/execution/SpawnExecution.ts: Pass undefined to getSpawn() when isRandomSpawn() is true, ignoring any client-specified tile
  • tests/core/execution/SpawnExecution.test.ts: Added test verifying that a client-specified tile is ignored when random spawn is enabled

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

FloPinguin

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. πŸŽ‰

ℹ️ Recent review info
βš™οΈ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c6b42fd-73b3-40ae-ac92-1ded3825a09f

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 95171fd and 0761607.

πŸ“’ Files selected for processing (2)
  • src/core/execution/SpawnExecution.ts
  • tests/core/execution/SpawnExecution.test.ts

Walkthrough

SpawnExecution.tick() now checks config().isRandomSpawn() and passes undefined to getSpawn when true, letting the server pick a random center. When false, it passes this.tile as before. A new test confirms a malicious client-specified tile is ignored under randomSpawn: true.

Changes

Random Spawn Fix

Layer / File(s) Summary
Random spawn logic and test
src/core/execution/SpawnExecution.ts, tests/core/execution/SpawnExecution.test.ts
tick() passes undefined to getSpawn when isRandomSpawn() is true; new test checks the player lands on a different, valid land tile when a malicious tile is given.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🎲 The client said "spawn here, right there!"
But the server just laughed in the air.
With undefined passed,
The cheat couldn't last β€”
A random safe tile, fair and square! πŸ—ΊοΈ

πŸš₯ Pre-merge checks | βœ… 5
βœ… Passed checks (5 passed)
Check name Status Explanation
Title check βœ… Passed The title clearly summarizes the main change: blocking clients from overriding random spawn selection.
Description check βœ… Passed The description matches the code changes and test coverage for random spawn selection.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check βœ… Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check βœ… Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❀️ Share

Comment @coderabbitai help to get the list of available commands.

@evanpelle evanpelle merged commit 71d70df into main Jun 27, 2026
18 checks passed
@evanpelle evanpelle deleted the fix/random-spawn-bypass branch June 27, 2026 18:10
@github-project-automation github-project-automation Bot moved this from Triage to Complete in OpenFront Release Management Jun 27, 2026
evanpelle pushed a commit that referenced this pull request Jun 27, 2026
## Description:

When random spawn mode is active, players are supposed to receive
randomly chosen spawns rather than choosing their own. However,
`SpawnExecution.getSpawn()` checks `center !== undefined` first, which
means if a player manually injects coordinates into the spawn intent
(bypassing the client-side UI guard), the random selection logic is
completely bypassed and the player gets their chosen coordinates.

This was fully exploitable in singleplayer (where no pre-created
`SpawnExecution` objects exist) and was a defense-in-depth gap in
multiplayer (relying on execution order of pre-created spawns to block
it via the `hasSpawned()` guard).

The fix forces `center` to `undefined` in `getSpawn()` when random
spawns are enabled, ensuring the random selection code path is always
taken regardless of what the client sends.

## Changes:
- `src/core/execution/SpawnExecution.ts`: Pass `undefined` to
`getSpawn()` when `isRandomSpawn()` is true, ignoring any
client-specified tile
- `tests/core/execution/SpawnExecution.test.ts`: Added test verifying
that a client-specified tile is ignored when random spawn is enabled

## Please complete the following:

- [X] I have added screenshots for all UI updates
- [X] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [X] I have added relevant tests to the test directory

## Please put your Discord username so you can be contacted if a bug or
regression is found:

FloPinguin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bugfix Fixes a bug

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

2 participants